-
-
Notifications
You must be signed in to change notification settings - Fork 478
refactor: Upgrade utils and replace useMergedState #1162
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: Upgrade utils and replace useMergedState #1162
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough将组件内部若干受控/非受控状态管理钩子从 useMergedState 切换为 useControlledState;同时将依赖 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as 用户
participant BaseSelect as BaseSelect
participant OpenHook as useControlledState
Note over BaseSelect,OpenHook: 展开态支持受控(open)与非受控(defaultOpen)两种模式
User->>BaseSelect: 点击打开/关闭
BaseSelect->>OpenHook: setInnerOpen(next)
alt open 由外部传入(受控)
Note over BaseSelect: 渲染受 props.open 驱动的状态
else 非受控
Note over BaseSelect: 由内部 state 更新并渲染
end
BaseSelect->>User: 更新下拉可见性(mergedOpen)
sequenceDiagram
autonumber
actor User as 用户
participant Select as Select
participant SearchHook as useControlledState
participant ValueHook as useControlledState
Note over Select: 搜索值与内部值均支持受控/非受控模式
User->>Select: 输入搜索或选择项
Select->>SearchHook: setSearchValue(input)
Select->>ValueHook: setInternalValue(selection)
alt props 提供值(受控)
Note over Select: 使用 props.searchValue / props.value 驱动展示
else 非受控
Note over Select: 使用内部 state 驱动展示
end
Select->>User: 更新 UI 与选择结果
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @EmilyyyLiu, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
此拉取请求旨在通过升级核心工具库并替换旧的状态管理钩子,来改进项目中的状态处理机制。这一系列更改有助于提高代码的可维护性和一致性,确保组件能够更有效地响应外部控制和内部状态变化。
Highlights
- @rc-component/util 依赖升级: 将
@rc-component/util
库的版本从^1.2.1
升级到^1.3.0
。 - 替换 useMergedState 为 useControlledState: 在
src/BaseSelect/index.tsx
和src/Select.tsx
文件中,将useMergedState
钩子替换为新的useControlledState
钩子,以优化组件内部状态的管理方式。
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request upgrades the @rc-component/util
dependency and replaces useMergedState
with useControlledState
in BaseSelect
and Select
components. The changes aim to improve state management and maintain component functionality. I have identified some areas for improvement, mainly focusing on ensuring the correct usage of useControlledState
and dependency updates.
import cls from 'classnames'; | ||
import useLayoutEffect from '@rc-component/util/lib/hooks/useLayoutEffect'; | ||
import useMergedState from '@rc-component/util/lib/hooks/useMergedState'; | ||
import useControlledState from '@rc-component/util/lib/hooks/useControlledState'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"@rc-component/trigger": "^3.0.0", | ||
"@rc-component/motion": "^1.1.4", | ||
"@rc-component/util": "^1.2.1", | ||
"@rc-component/util": "^1.3.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defaultValue: defaultOpen, | ||
value: open, | ||
}); | ||
const [innerOpen, setInnerOpen] = useControlledState<boolean>(defaultOpen || false, open); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
在原本 useMergedState 的使用中 defaultOpen 的使用优先于默认的 false
src/Select.tsx
Outdated
value: searchValue, | ||
postState: (search) => search || '', | ||
}); | ||
const [mergedSearchValue, setSearchValue] = useControlledState('', searchValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里要额外写一下:
const [internalSearchValue, setSearchValue] = useControlledState('', searchValue);
const mergedSearchValue = internalSearchValue || '';
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1162 +/- ##
=======================================
Coverage 98.11% 98.11%
=======================================
Files 39 39
Lines 1489 1489
Branches 423 425 +2
=======================================
Hits 1461 1461
Misses 28 28 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/BaseSelect/index.tsx (1)
386-386
: 行为差异提醒:useMergedState → useControlledState 可能限制“受控/非受控切换”
- useControlledState 通常在首渲染锁定“是否受控”,后续不鼓励从非受控切换为受控(或反之)。若外部对
open
存在动态切换(从undefined
变为boolean
或相反),可能与旧实现的宽松行为不同,产生不可预期状态。- 当前初始化
defaultOpen || false
语义与旧逻辑等价,OK。为降低误用风险,可在开发环境做一次警告封装(仅示意):
-const [innerOpen, setInnerOpen] = useControlledState<boolean>(defaultOpen || false, open); +const [innerOpen, setInnerOpen] = useControlledState<boolean>(defaultOpen || false, open); +if (process.env.NODE_ENV !== 'production') { + const controlledRef = React.useRef(open !== undefined); + React.useEffect(() => { + if (controlledRef.current !== (open !== undefined)) { + // eslint-disable-next-line no-console + console.warn('[BaseSelect] `open` 从受控切换到非受控(或相反)不被支持,请固定用法。'); + } + }, [open]); +}另:微建议将
defaultOpen || false
改为更直观的Boolean(defaultOpen)
。请补一例用例覆盖:
- 仅传
defaultOpen
(非受控);- 传
open
(受控);- 模拟从未传
open
→ 传入open
的切换(应得到预期警告/行为)。src/Select.tsx (1)
275-276
: 移除 postState 后的空值归一化差异:请确认不会传入 null原先
useMergedState('', { value: searchValue, postState: v => v || '' })
会把undefined/null
归一为''
。现在useControlledState('', searchValue)
若外部误传null
,mergedSearchValue
可能为null
,进而影响字符串操作与过滤逻辑。若希望保持兼容,可在取值处做一次兜底归一(不改变受控判定):
-const [mergedSearchValue, setSearchValue] = useControlledState('', searchValue); +const normalizedSearchValue = (searchValue as string | undefined) ?? ''; +const [mergedSearchValue, setSearchValue] = useControlledState('', normalizedSearchValue);或者在使用点统一
(mergedSearchValue || '')
。请结合现有用例选择其一,并补充测试覆盖searchValue: undefined/null/''
。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package.json
(1 hunks)src/BaseSelect/index.tsx
(2 hunks)src/Select.tsx
(3 hunks)
🔇 Additional comments (4)
package.json (1)
54-54
: 校验 util v1.3.0 导出行为与用法一致
- 确认项目依赖解析的
@rc-component/util
版本 ≥ 1.3.0(若仓库未包含锁文件,请根据 package.json 或安装工具手动检查)。- 检查
node_modules/@rc-component/util/lib/hooks/useControlledState
是否为 default export,且其签名与在src/Select.tsx
(第 32 行)和src/BaseSelect/index.tsx
(第 4 行)中的用法保持一致。- 发布包在 ESM/CJS 导出上可能存在差异,打包后务必验证运行时兼容性。
src/BaseSelect/index.tsx (1)
4-4
: 改为使用 useControlledState:导入路径与默认导出需与 [email protected] 对齐导入改为默认导出符合 [email protected] 的“default export”调整,但仍建议编译验证以防导出名与形态不一致导致运行时错误。(github.com)
src/Select.tsx (2)
32-32
: 改为 useControlledState:导入层面无问题,但请确保 util 导出兼容与 BaseSelect 同步注意点:确认默认导出与签名一致,避免构建期/运行期异常。
343-344
: 值状态迁移为 useControlledState:语义对齐,注意受控模式下 setInternalValue 为 no-op此改动与组件受控/非受控设计契合;在受控模式下
setInternalValue
将 no-op,后续变更由外部value
驱动,符合预期。建议新增/回归测试覆盖:
- multiple 模式下
value=null
与[]
的显示行为(你已在后续用internalValue===null ? [] : internalValue
兜底,验证一下可避免回归)。
关联issue:ant-design/ant-design#54854
替换 useMergedState 为 useControlledState
Summary by CodeRabbit
新特性
重构
杂务